Skip to content

Testing concurrent reads and writes#83

Closed
icristescu wants to merge 1 commit intomirage:masterfrom
icristescu:co_test
Closed

Testing concurrent reads and writes#83
icristescu wants to merge 1 commit intomirage:masterfrom
icristescu:co_test

Conversation

@icristescu
Copy link
Contributor

No description provided.


let tbl = tbl index_name

let w = Index.v ~fresh:false ~log_size index_name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer constructing this inside the test (with a subscoped directory name), along with the pipes below, and passing them explicitly to the helper functions that need them.

These sorts of global test instances seem to lead to a mess 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, but for now, it still depends on the index being populated by the tbl in common.

index_name is defined above and subscoped to _tests. Is this what you meant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this test uses index_name to construct it's instance-under-test, future tests will also test the same instance on disk. I was suggesting doing something like this to make sure each test uses a unique name.

It's not a big deal, however, since my PR will clean up these sorts of details.

Copy link
Member

@craigfe craigfe Sep 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And w.r.t. the table being populated by Common: I'm also working on that 🙂

@icristescu icristescu force-pushed the co_test branch 3 times, most recently from b1d0f37 to ca85d57 Compare September 10, 2019 15:06
@icristescu icristescu mentioned this pull request Sep 11, 2019
@icristescu icristescu force-pushed the co_test branch 3 times, most recently from 7e915e1 to ae7a216 Compare September 11, 2019 10:06
@icristescu
Copy link
Contributor Author

Merged the concurrent tests in a single file in PR #66.

@icristescu icristescu closed this Sep 12, 2019
@icristescu icristescu deleted the co_test branch September 12, 2019 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants